-
Notifications
You must be signed in to change notification settings - Fork 72
[DO NOT MERGE YET, see NOTE] Account Details: Remove the feature flag to open up this feature to all sites #11170
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: 0 B Total Size: 877 kB ℹ️ View Unchanged
|
…detailsenabled-flag-on-the-client-side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No major concerns. However, the change on the JS side means that we need to re-compile the JS before issuing a patch.
To truly have a minimal change, we could also set the default value returned in WC_Payments_Features to 1. At least, that's how we've enabled feature flags in the past ( example ).
Changing the default value on the PHP side of things also allows for a per-merchant rollback in case of issues, since a WP filter can intercept the value.
add_filter( 'pre_option__wcpay_feature_account_details', function ( $v ) {
// ensures that once the next version of WCPay is released, this filter can be automatically ignored
if ( version_compare( WCPAY_VERSION_NUMBER, '10.6.0, '<' ) ) {
return '0';
}
return $v;
}, 100 );What do you think?
…detailsenabled-flag-on-the-client-side
|
@frosso - thanks for your suggestion. I like that idea, and I've updated the PR to follow that. Ready for another review. Edit: tests are failing; I will need to fix them |
frosso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for entertaining my suggestions! It looks like the failing test is WC_Payments_Features_Test - probably just needs to have a default value set for the feature flag in the test setup phase :)
Claude gave me a complex fix but then I found out this is much better and reflects what we're having, i.e. Here is sthe commit d400920 |
frosso
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still looking great! 💪
…detailsenabled-flag-on-the-client-side
…detailsenabled-flag-on-the-client-side
Note
This PR should be only merged into version 10.5.0, i.e. after Dec 10, 2025 (10.4.0 RC Freeze date).
Fixes WOOPMNT-5528
Changes proposed in this Pull Request
Testing instructions
wp option delete _wcpay_feature_account_detailsACCOUNT_DETAILS_ENABLED.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge